CSSTUDIO-3420 Performance improvements to the Waterfall Plot widget#3544
Conversation
…remove synchronization on pvData.
…> for representing waveforms.
…eToInstantToValue'.
|
|
|
@kasemir Thanks for the tip! |
|
@kasemir, |
|
@georgweiss In the best case (which, e.g., is the case in this pull request), the size is known in advance. However, the copying is usually not a problem since it occurs relatively seldomly: suppose an array is initialized with size 1, and that 2^n values are written to it. Then the number N of write operations to arrays is (including the copying): N = 1 + 2 + 4 + 8 + 16 + ... + 2^n = 2*2^n - 1 If the size is known in advance, then the number of write operations is 2^n. Therefore, by using this strategy, the number of write operations to arrays is only doubled in comparison to the case where the size is known in advance. |
|
@abrahamwolk, if the size is known in advance and does not change, why not use |
|
@georgweiss I think it would be possible to use |
|
If efficiency is of importance, then I think readability would be a cheap price to pay. For reference: for Android Google recommends arrays (primitives or objects) over higher level abstractions where possible. |
|
I think it's certainly a possible optimization, and I think it's plausible that it may turn out to be a good optimization to implement. However, there may be other optimizations also, and at this stage I prefer to focus on higher level optimizations. I see it as a trade-off between readability, maintainability, and the ability to modify the code on the one hand, and efficiency on the other hand. In particular, implementing lower level optimizations may make it more difficult to implement higher level optimizations, and for this reason I think that it is good to first focus on the high-level abstractions. If performance is still an issue after the high-level structure is optimized, then I think one should consider low-level optimizations. |
3 participants
This pull request implements performance improvements to the Waterfall Plot widget:
TreeMapwith manual synchronization (which seems to have had a bug, and was apparently not working as intended!) to instead using aConcurrentSkipListMap, with the consequence that manual synchronization is no longer needed (since the necessary synchronization is instead handled by the data structure).LinkedList(with random-accesses taking linear time) to represent waveform data,ArrayListis used so that random-accesses take constant time.pvNameToInstantToValuewas also changed fromLinkedListtoArrayList, but this probably doesn't have a noticeable effect for practical use cases, where the number of PVs is expected to be small.